revision: fix --no-walk path filtering regression#2181
Conversation
7645e7c to
c5d7ae3
Compare
Since dd4bc01 (revision: use priority queue for non-limited streaming walks, 2026-05-27), "git rev-list --no-walk <commit> -- <path>" ignores the path arguments and outputs all commits regardless of whether they touch the given paths. That commit introduced a REV_WALK_NO_WALK enum value to separate --no-walk from the streaming walk in get_revision_1(). The new case skips process_parents(), which is correct for not enqueuing parents, but also skips try_to_simplify_commit() which process_parents() calls to evaluate whether each commit touches the given paths. Add a call to try_to_simplify_commit() for the REV_WALK_NO_WALK case, folding it into the existing REV_WALK_REFLOG case which already does the same. Add tests for --no-walk path filtering to t6017. The "single commit, match" test is defensive and passes without the fix, while the other two fail without it. Reported-by: Peter Colberg <pcolberg@redhat.com> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
c5d7ae3 to
d03a63d
Compare
|
/submit |
|
Submitted as pull.2181.git.1784198879711.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Kristofer Karlsson <krka@spotify.com>
>
> Since dd4bc01c0a (revision: use priority queue for non-limited
> streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
> -- <path>" ignores the path arguments and outputs all commits
> regardless of whether they touch the given paths.
>
> That commit introduced a REV_WALK_NO_WALK enum value to separate
> --no-walk from the streaming walk in get_revision_1(). The new
> case skips process_parents(), which is correct for not enqueuing
> parents, but also skips try_to_simplify_commit() which
> process_parents() calls to evaluate whether each commit touches
> the given paths.
>
> Add a call to try_to_simplify_commit() for the
> REV_WALK_NO_WALK case, folding it into the existing
> REV_WALK_REFLOG case which already does the same.
>
> Add tests for --no-walk path filtering to t6017. The
> "single commit, match" test is defensive and passes without
> the fix, while the other two fail without it.
>
> Reported-by: Peter Colberg <pcolberg@redhat.com>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
> revision: fix --no-walk path filtering regression
>
> Fix for a regression reported by Peter Colberg [1] where git rev-list
> --no-walk <commit> -- <path> ignores path arguments since dd4bc01c0a.
>
> Verified against linux.git with the exact example from the report:
>
> git rev-list --topo-order v7.0..v7.1 -- drivers/gpu/drm/ |
> git rev-list --stdin --no-walk=unsorted -- ':!drivers/gpu/drm/'
>
>
> Without fix: 2026 commits (all pass through unfiltered) With fix: 146
> commits (correctly filtered)
>
> [1]
> https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u
OK. Without this fix, a commit that applied this patch (which does
not touch Makefile) shows the commit message in
$ git show HEAD -- Makefile
but with this fix, just like Git 2.54 did, the command stays silent.
Will queue. Thanks. |
|
Peter Colberg wrote on the Git mailing list (how to reply to this email): Hi Kristofer,
On Thu, Jul 16, 2026 at 10:47:58AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
>
> Since dd4bc01c0a (revision: use priority queue for non-limited
> streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
> -- <path>" ignores the path arguments and outputs all commits
> regardless of whether they touch the given paths.
>
> That commit introduced a REV_WALK_NO_WALK enum value to separate
> --no-walk from the streaming walk in get_revision_1(). The new
> case skips process_parents(), which is correct for not enqueuing
> parents, but also skips try_to_simplify_commit() which
> process_parents() calls to evaluate whether each commit touches
> the given paths.
>
> Add a call to try_to_simplify_commit() for the
> REV_WALK_NO_WALK case, folding it into the existing
> REV_WALK_REFLOG case which already does the same.
>
> Add tests for --no-walk path filtering to t6017. The
> "single commit, match" test is defensive and passes without
> the fix, while the other two fail without it.
>
> Reported-by: Peter Colberg <pcolberg@redhat.com>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
Thank you very much for the fix, which passes as well for my use case.
Peter
> ---
> revision: fix --no-walk path filtering regression
>
> Fix for a regression reported by Peter Colberg [1] where git rev-list
> --no-walk <commit> -- <path> ignores path arguments since dd4bc01c0a.
>
> Verified against linux.git with the exact example from the report:
>
> git rev-list --topo-order v7.0..v7.1 -- drivers/gpu/drm/ |
> git rev-list --stdin --no-walk=unsorted -- ':!drivers/gpu/drm/'
>
>
> Without fix: 2026 commits (all pass through unfiltered) With fix: 146
> commits (correctly filtered)
>
> [1]
> https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2181%2Fspkrka%2Fkk%2Fno-walk-pathspec-fix-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2181/spkrka/kk/no-walk-pathspec-fix-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2181
>
> revision.c | 2 +-
> t/t6017-rev-list-stdin.sh | 18 ++++++++++++++++++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/revision.c b/revision.c
> index ccbe2e03d1..e990e3f96b 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -4419,6 +4419,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
>
> switch (mode) {
> case REV_WALK_REFLOG:
> + case REV_WALK_NO_WALK:
> try_to_simplify_commit(revs, commit);
> break;
> case REV_WALK_TOPO:
> @@ -4432,7 +4433,6 @@ static struct commit *get_revision_1(struct rev_info *revs)
> oid_to_hex(&commit->object.oid));
> }
> break;
> - case REV_WALK_NO_WALK:
> case REV_WALK_LIMITED:
> break;
> }
> diff --git a/t/t6017-rev-list-stdin.sh b/t/t6017-rev-list-stdin.sh
> index 4821b90e74..32284f1831 100755
> --- a/t/t6017-rev-list-stdin.sh
> +++ b/t/t6017-rev-list-stdin.sh
> @@ -148,4 +148,22 @@ test_expect_success '--not via stdin does not influence revisions from command l
> test_cmp expect actual
> '
>
> +test_expect_success '--no-walk filters by path (single commit, match)' '
> + git rev-parse side-1 >expect &&
> + git rev-list --no-walk side-1 -- file-1 >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success '--no-walk filters by path (single commit, no match)' '
> + git rev-list --no-walk side-2 -- file-1 >actual &&
> + test_must_be_empty actual
> +'
> +
> +test_expect_success '--no-walk with pathspec exclusion' '
> + git rev-parse side-3 side-2 >expect &&
> + git rev-parse side-1 side-2 side-3 >input &&
> + git rev-list --stdin --no-walk -- ":!file-1" <input >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
>
> base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
> --
> gitgitgadget
>
|
|
This patch series was integrated into seen via git@49c2f60. |
|
This branch is now known as |
|
This patch series was integrated into next via git@4dd6fb0. |
|
There was a status update in the "Cooking" section about the branch The 'git rev-list --no-walk' command lost pathspec filtering when the streaming walk was refactored, which has been corrected. Will merge to 'master'. source: <pull.2181.git.1784198879711.gitgitgadget@gmail.com> |
|
There was a status update in the "Cooking" section about the branch The 'git rev-list --no-walk' command has been corrected to restore pathspec filtering, which was lost when the streaming walk was refactored. Will merge to 'master'. source: <pull.2181.git.1784198879711.gitgitgadget@gmail.com> |
|
This patch series was integrated into master via git@60aa81f. |
|
Congratulations! 🎉 Your patch series was merged into upstream via 60aa81f. Note: this pull request will show as "Closed" rather than "Merged" because the merge happened in the upstream repository, not on GitHub. This is expected — your contribution has been accepted! |
Fix for a regression reported by Peter Colberg [1] where
git rev-list --no-walk <commit> -- <path>ignores path argumentssince dd4bc01.
Verified against linux.git with the exact example from the report:
Without fix: 2026 commits (all pass through unfiltered)
With fix: 146 commits (correctly filtered)
[1] https://lore.kernel.org/git/CAL71e4NjDTHbKR8z7pSrPpzDrX19JOTR04sArm7P=m5ivqkskA@mail.gmail.com/T/#u
CC: Peter Colberg pcolberg@redhat.com